stabilize ptr::swap_nonoverlapping in const #137280

RalfJung · 2025-02-19T16:58:32Z

The blocking issue mentioned there is resolved by documentation. We may in the future actually support such code, but that is blocked on rust-lang/const-eval#72 which is non-trivial to implement. Meanwhile, this completes stabilization of all const fn in ptr. :)

Here's a version of the problematic example to play around with:
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=6c390452379fb593e109b8f8ee854d2a

Should be FCP'd with both @rust-lang/libs-api and @rust-lang/lang since swap_nonoverlapping is documented to work as an "untyped" operation but due to the limitation mentioned above, that's not entirely true during const evaluation. I expect this limitation will only be hit in niche corner cases, so the benefits of having this function work most of the time outweigh the downsides of users running into this problem. (Note that unsafe code could already hit this limitation before this PR by doing cursed pointer casts, but having it hidden inside swap_nonoverlapping feels a bit different.)

rustbot · 2025-02-19T16:58:42Z

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-02-19T16:58:45Z

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @rust-lang/wg-const-eval

scottmcm · 2025-02-19T21:53:24Z

library/core/src/ptr/mod.rs

+/// # Const evaluation limitations
+///
+/// If this function is invoked during const-evaluation, the current implementation has a small (and
+/// rarely relevant) limitation: if `count` is at least 2 and the data pointed to by `x` or `y`


I suppose it's a "current" thing not a promise, but specifically things like "at least 2" seem oddly specific.

Are we sure it's impossible to, say, create a failing version using a big packed struct that arranges to have an unaligned pointer that carries provenance but is unaligned so that it's copied in multiple parts by the implementation?

(I don't consider this an FCP blocker.)

Yes we are. The const implementation will always swap entire T at once (this got fixed in #134689). We even have a test covering this:

rust/library/coretests/tests/ptr.rs

Lines 905 to 963 in 749c3e6

fn test_const_swap_ptr() {

// The `swap` functions are implemented in the library, they are not primitives.

// Only `swap_nonoverlapping` takes a count; pointers that cross multiple elements

// are *not* supported.

// We put the pointer at an odd offset in the type and copy them as an array of bytes,

// which should catch most of the ways that the library implementation can get it wrong.

#[cfg(target_pointer_width = "32")]

type HalfPtr = i16;

#[cfg(target_pointer_width = "64")]

type HalfPtr = i32;

#[repr(C, packed)]

#[allow(unused)]

struct S {

f1: HalfPtr,

// Crucially this field is at an offset that is not a multiple of the pointer size.

ptr: &'static i32,

// Make sure the entire type does not have a power-of-2 size:

// make it 3 pointers in size. This used to hit a bug in `swap_nonoverlapping`.

f2: [HalfPtr; 3],

}

// Ensure the entire thing is usize-aligned, so in principle this

// looks like it could be eligible for a `usize` copying loop.

#[cfg_attr(target_pointer_width = "32", repr(align(4)))]

#[cfg_attr(target_pointer_width = "64", repr(align(8)))]

struct A(S);

const {

let mut s1 = A(S { ptr: &1, f1: 0, f2: [0; 3] });

let mut s2 = A(S { ptr: &666, f1: 0, f2: [0; 3] });

// Swap ptr1 and ptr2, as an array.

type T = [u8; mem::size_of::<A>()];

unsafe {

ptr::swap(ptr::from_mut(&mut s1).cast::<T>(), ptr::from_mut(&mut s2).cast::<T>());

}

// Make sure they still work.

assert!(*s1.0.ptr == 666);

assert!(*s2.0.ptr == 1);

// Swap them back, again as an array.

// (This is where we'd use a `u8` type and a `count` of `size_of::<T>()` if

// it were not for the limitation of `swap_nonoverlapping` around pointers crossing

// multiple elements.)

unsafe {

ptr::swap_nonoverlapping(

ptr::from_mut(&mut s1).cast::<T>(),

ptr::from_mut(&mut s2).cast::<T>(),

1,

);

}

// Make sure they still work.

assert!(*s1.0.ptr == 1);

assert!(*s2.0.ptr == 666);

};

"at least 2" is technically redundant since the text already says a pointer must span the boundary between two chunks being swapped, and if there's only one chunk there is no boundary. But I felt it helped make the point more clear.

traviscross · 2025-02-19T22:16:49Z

@rfcbot fcp merge

rfcbot · 2025-02-19T22:16:52Z

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

BurntSushi · 2025-03-19T17:07:55Z

library/core/src/ptr/mod.rs

+///     let ptr = data2.as_ptr().cast::<*const i32>().read_unaligned();
+///     assert!(*ptr == 42);
+/// } }
+/// ```


Can you explicitly state that this may change in the future somewhere in the docs? I know the "current" wording seemingly implies that, but I'd rather do some very clear CYA here.

Also, the docs here seem to leave it a bit mysterious as to what "fail" means. Is it a panic? Silent logical error? Something else? Could that be clarified as well?

+1 for the last bit; saying "compilation may fail" or similar would be clearer.

Can you explicitly state that this may change in the future somewhere in the docs? I know the "current" wording seemingly implies that, but I'd rather do some very clear CYA here.

Okay, done.

Also, the docs here seem to leave it a bit mysterious as to what "fail" means. Is it a panic? Silent logical error? Something else? Could that be clarified as well?

It's a standard const-eval failure. I can say "fail to evaluate", not sure if that would help. I'm a bit worried if I say "compilation will fail" people think this is some sort of static analysis, when it is a runtime check -- but it's "at runtime of const-eval code". (So if a problematic call is in dead code, or you write a generic associated const that is never evaluated, compilation will still succeed.)

For is_null we said that the function panics in some cases in const. That's not 100% correct, but a const-eval failure is basically equivalent to a panic during const-eval.

@tmandry @BurntSushi could you take a look at the current version?

tmandry · 2025-04-18T23:46:18Z

@rfcbot reviewed

I'm not too concerned about the limitation. Treating pointer-containing memory as bytes doesn't seem like it should be encouraged anyway, and this also looks straightforward to work around.

rfcbot · 2025-04-18T23:46:24Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot · 2025-04-28T23:49:23Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

RalfJung · 2025-04-29T08:40:37Z

@joboet this is ready for review :)
@rustbot ready

RalfJung · 2025-05-04T14:15:28Z

The review here is mostly about the const-eval-specific doc comment... so @rust-lang/wg-const-eval maybe one of you could take this?

lcnr · 2025-05-04T15:38:28Z

r? lcnr

@bors r+ rollup

bors · 2025-05-04T15:38:31Z

📌 Commit 9f4abd3 has been approved by lcnr

It is now in the queue for this repository.

Rollup of 6 pull requests Successful merges: - rust-lang#137280 (stabilize ptr::swap_nonoverlapping in const) - rust-lang#140457 (Use target-cpu=z13 on s390x codegen const vector test) - rust-lang#140619 (Small adjustments to `check_attribute_safety` to make the logic more obvious) - rust-lang#140625 (Suggest `retain_mut` over `retain` as `Vec::extract_if` alternative) - rust-lang#140627 (Allow linking rustc and rustdoc against the same single tracing crate) - rust-lang#140630 (Async drop source info fix for proxy-drop-coroutine) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137280 - RalfJung:const_swap_nonoverlapping, r=lcnr stabilize ptr::swap_nonoverlapping in const Closes rust-lang#133668 The blocking issue mentioned there is resolved by documentation. We may in the future actually support such code, but that is blocked on rust-lang/const-eval#72 which is non-trivial to implement. Meanwhile, this completes stabilization of all `const fn` in `ptr`. :) Here's a version of the problematic example to play around with: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=6c390452379fb593e109b8f8ee854d2a Should be FCP'd with both `@rust-lang/libs-api` and `@rust-lang/lang` since `swap_nonoverlapping` is documented to work as an "untyped" operation but due to the limitation mentioned above, that's not entirely true during const evaluation. I expect this limitation will only be hit in niche corner cases, so the benefits of having this function work most of the time outweigh the downsides of users running into this problem. (Note that unsafe code could already hit this limitation before this PR by doing cursed pointer casts, but having it hidden inside `swap_nonoverlapping` feels a bit different.)

rustbot assigned joboet Feb 19, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 19, 2025

RalfJung added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Feb 19, 2025

RalfJung mentioned this pull request Feb 19, 2025

Tracking Issue for const_swap_nonoverlapping #133668

Closed

4 tasks

RalfJung force-pushed the const_swap_nonoverlapping branch from c820135 to 749c3e6 Compare February 19, 2025 17:17

traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 19, 2025

This comment was marked as duplicate.

Sign in to view

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 19, 2025

scottmcm reviewed Feb 19, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 19, 2025

the8472 added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 19, 2025

This comment was marked as resolved.

Sign in to view

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 19, 2025

RalfJung force-pushed the const_swap_nonoverlapping branch from 749c3e6 to 99480fc Compare February 20, 2025 07:35

BurntSushi reviewed Mar 19, 2025

View reviewed changes

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Apr 18, 2025

RalfJung force-pushed the const_swap_nonoverlapping branch 2 times, most recently from cf4cce0 to 4c40780 Compare April 20, 2025 09:44

rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 28, 2025

rfcbot added the to-announce Announce this issue on triage meeting label Apr 28, 2025

rustbot mentioned this pull request Apr 28, 2025

Tracking issue for release notes of #137280: stabilize ptr::swap_nonoverlapping in const #140416

Open

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2025

stabilize ptr::swap_nonoverlapping in const

9f4abd3

RalfJung force-pushed the const_swap_nonoverlapping branch from 4c40780 to 9f4abd3 Compare April 29, 2025 08:41

RalfJung removed the S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. label Apr 29, 2025

rustbot assigned lcnr May 4, 2025

rustbot unassigned joboet May 4, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 4, 2025

tgross35 mentioned this pull request May 4, 2025

Rollup of 6 pull requests #140646

Merged

bors merged commit fcb9da5 into rust-lang:master May 5, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone May 5, 2025

RalfJung deleted the const_swap_nonoverlapping branch May 5, 2025 13:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stabilize ptr::swap_nonoverlapping in const #137280

stabilize ptr::swap_nonoverlapping in const #137280

RalfJung commented Feb 19, 2025 •

edited

Loading

rustbot commented Feb 19, 2025

rustbot commented Feb 19, 2025

This comment was marked as duplicate.

This comment was marked as duplicate.

scottmcm Feb 19, 2025

RalfJung Feb 20, 2025 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

traviscross commented Feb 19, 2025

rfcbot commented Feb 19, 2025 •

edited

Loading

BurntSushi Mar 19, 2025

tmandry Apr 18, 2025

RalfJung Apr 20, 2025 •

edited

Loading

RalfJung Apr 20, 2025 •

edited

Loading

RalfJung Apr 29, 2025

tmandry commented Apr 18, 2025

rfcbot commented Apr 18, 2025

rfcbot commented Apr 28, 2025

RalfJung commented Apr 29, 2025

RalfJung commented May 4, 2025

lcnr commented May 4, 2025

bors commented May 4, 2025

	fn test_const_swap_ptr() {
	// The `swap` functions are implemented in the library, they are not primitives.
	// Only `swap_nonoverlapping` takes a count; pointers that cross multiple elements
	// are not supported.
	// We put the pointer at an odd offset in the type and copy them as an array of bytes,
	// which should catch most of the ways that the library implementation can get it wrong.

	#[cfg(target_pointer_width = "32")]
	type HalfPtr = i16;
	#[cfg(target_pointer_width = "64")]
	type HalfPtr = i32;

	#[repr(C, packed)]
	#[allow(unused)]
	struct S {
	f1: HalfPtr,
	// Crucially this field is at an offset that is not a multiple of the pointer size.
	ptr: &'static i32,
	// Make sure the entire type does not have a power-of-2 size:
	// make it 3 pointers in size. This used to hit a bug in `swap_nonoverlapping`.
	f2: [HalfPtr; 3],
	}

	// Ensure the entire thing is usize-aligned, so in principle this
	// looks like it could be eligible for a `usize` copying loop.
	#[cfg_attr(target_pointer_width = "32", repr(align(4)))]
	#[cfg_attr(target_pointer_width = "64", repr(align(8)))]
	struct A(S);

	const {
	let mut s1 = A(S { ptr: &1, f1: 0, f2: [0; 3] });
	let mut s2 = A(S { ptr: &666, f1: 0, f2: [0; 3] });

	// Swap ptr1 and ptr2, as an array.
	type T = [u8; mem::size_of::<A>()];
	unsafe {
	ptr::swap(ptr::from_mut(&mut s1).cast::<T>(), ptr::from_mut(&mut s2).cast::<T>());
	}

	// Make sure they still work.
	assert!(*s1.0.ptr == 666);
	assert!(*s2.0.ptr == 1);

	// Swap them back, again as an array.
	// (This is where we'd use a `u8` type and a `count` of `size_of::<T>()` if
	// it were not for the limitation of `swap_nonoverlapping` around pointers crossing
	// multiple elements.)
	unsafe {
	ptr::swap_nonoverlapping(
	ptr::from_mut(&mut s1).cast::<T>(),
	ptr::from_mut(&mut s2).cast::<T>(),
	1,
	);
	}

	// Make sure they still work.
	assert!(*s1.0.ptr == 1);
	assert!(*s2.0.ptr == 666);
	};

stabilize ptr::swap_nonoverlapping in const #137280

stabilize ptr::swap_nonoverlapping in const #137280

Conversation

RalfJung commented Feb 19, 2025 • edited Loading

rustbot commented Feb 19, 2025

rustbot commented Feb 19, 2025

This comment was marked as duplicate.

This comment was marked as duplicate.

scottmcm Feb 19, 2025

Choose a reason for hiding this comment

RalfJung Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

traviscross commented Feb 19, 2025

rfcbot commented Feb 19, 2025 • edited Loading

BurntSushi Mar 19, 2025

Choose a reason for hiding this comment

tmandry Apr 18, 2025

Choose a reason for hiding this comment

RalfJung Apr 20, 2025 • edited Loading

Choose a reason for hiding this comment

RalfJung Apr 20, 2025 • edited Loading

Choose a reason for hiding this comment

RalfJung Apr 29, 2025

Choose a reason for hiding this comment

tmandry commented Apr 18, 2025

rfcbot commented Apr 18, 2025

rfcbot commented Apr 28, 2025

RalfJung commented Apr 29, 2025

RalfJung commented May 4, 2025

lcnr commented May 4, 2025

bors commented May 4, 2025

RalfJung commented Feb 19, 2025 •

edited

Loading

RalfJung Feb 20, 2025 •

edited

Loading

rfcbot commented Feb 19, 2025 •

edited

Loading

RalfJung Apr 20, 2025 •

edited

Loading

RalfJung Apr 20, 2025 •

edited

Loading